Fixes #22479 - Handle remote directory with undefined parent #410
Conversation
| it { should contain_foreman_proxy__plugin('discovery') } | ||
| it { should contain_foreman_proxy__feature('Discovery') } | ||
| it { should_not contain_foreman_proxy__remote_file("#{tftproot}/boot/fdi-image-latest.tar") } | ||
| it { should_not contain_exec('untar fdi-image-latest.tar') } |
There was a problem hiding this comment.
kind of meta, but I think is_expected.to / is_expected.not_to is generally considered best practice these days.
There was a problem hiding this comment.
You're right. I copied the spec from puppet-foreman and didn't replace it.
There was a problem hiding this comment.
Correction: these was an existing file. I at least wanted to keep it consistent within this file.
manifests/remote_file.pp
Outdated
| creates => $parent, | ||
| } | ||
| -> exec {"retrieve_${title}": | ||
| command => "/usr/bin/curl -s ${remote_location} -o ${title}", |
There was a problem hiding this comment.
Is curl definitely going to be installed on all supported platforms?
There was a problem hiding this comment.
I've updated it to use the plain file resource for the download. Not sure how well that'll work in practice.
manifests/remote_file.pp
Outdated
| creates => $parent, | ||
| } | ||
| -> exec {"retrieve_${title}": | ||
| command => "/usr/bin/curl -s ${remote_location} -o ${title}", |
There was a problem hiding this comment.
want to add --connect-timeout foo since your setting timeout => 0, below?
There was a problem hiding this comment.
File doesn't expose a timeout setting so that may be tricky.
This can happen when tftp is set to false or unmanaged. We move the remote_file define to this module since puppet-foreman no longer uses it itself. That will allow us to remove it there in a next major release.
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
| creates => $parent, | ||
| } | ||
| -> file { $title: | ||
| source => $remote_location, |
There was a problem hiding this comment.
@ekohl if you don't mind can you copy the test from theforeman/puppet-foreman#627 ?
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
@sean797 Thanks! I modified it slightly and added a commit that enables acceptance tests. |
|
Rerunning to see if that helps |
|
Pushed a fix that was also done in the voxpupuli nodeset |
|
this still fails on Ubuntu? |
|
Yes, not sure what's going on there. Doesn't look like an actual test failure but rather the whole image. |
|
For now I just excluded the tests on Ubuntu because I couldn't figure it out. |
This can happen when tftp is set to false or unmanaged.
We move the remote_file define to this module since puppet-foreman no longer uses it itself. That will allow us to remove it there in a next major release.